Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

indi-full: Indi 3rdparty refactor #306650

Merged
merged 2 commits into from
Aug 25, 2024
Merged

indi-full: Indi 3rdparty refactor #306650

merged 2 commits into from
Aug 25, 2024

Conversation

returntoreality
Copy link
Contributor

@returntoreality returntoreality commented Apr 24, 2024 •
edited
Loading

Description of changes

This is refactoring the indi 3rdparty drivers into their own derivations instead of having the firmware and 3rdparty derivations. This also sets the architecture support for each driver and assign the correct licences (before the unfree binary blobs from the 3rdparty repo were not marked as unfree). indi-full is now containing only free drivers and there is a new indi-full-nonfree which also contains the nonfree drivers.
I also added myself to the maintainers.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@returntoreality
Copy link
Contributor Author

Result of nixpkgs-review run on x86_64-linux 1

56 packages built:
  • indi-full
  • indi-full-nonfree
  • indi3rdParty.indi-aagcloudwatcher-ng
  • indi3rdParty.indi-aok
  • indi3rdParty.indi-apogee
  • indi3rdParty.indi-armadillo-platypus
  • indi3rdParty.indi-asi
  • indi3rdParty.indi-astroasis
  • indi3rdParty.indi-astrolink4
  • indi3rdParty.indi-astromechfoc
  • indi3rdParty.indi-atik
  • indi3rdParty.indi-avalon
  • indi3rdParty.indi-avalonud
  • indi3rdParty.indi-beefocus
  • indi3rdParty.indi-bresserexos2
  • indi3rdParty.indi-celestronaux
  • indi3rdParty.indi-dreamfocuser
  • indi3rdParty.indi-dsi
  • indi3rdParty.indi-duino
  • indi3rdParty.indi-eqmod
  • indi3rdParty.indi-ffmv
  • indi3rdParty.indi-fishcamp
  • indi3rdParty.indi-fli
  • indi3rdParty.indi-gige
  • indi3rdParty.indi-gphoto
  • indi3rdParty.indi-gpsd
  • indi3rdParty.indi-gpsnmea
  • indi3rdParty.indi-inovaplx
  • indi3rdParty.indi-limesdr
  • indi3rdParty.indi-maxdomeii
  • indi3rdParty.indi-mgen
  • indi3rdParty.indi-mi
  • indi3rdParty.indi-nexdome
  • indi3rdParty.indi-nightscape
  • indi3rdParty.indi-nut
  • indi3rdParty.indi-ocs
  • indi3rdParty.indi-orion-ssg3
  • indi3rdParty.indi-pentax
  • indi3rdParty.indi-playerone
  • indi3rdParty.indi-qhy
  • indi3rdParty.indi-qsi
  • indi3rdParty.indi-rolloffino
  • indi3rdParty.indi-rtklib
  • indi3rdParty.indi-sbig
  • indi3rdParty.indi-shelyak
  • indi3rdParty.indi-spectracyber
  • indi3rdParty.indi-starbook
  • indi3rdParty.indi-starbook-ten
  • indi3rdParty.indi-svbony
  • indi3rdParty.indi-sx
  • indi3rdParty.indi-talon6
  • indi3rdParty.indi-toupbase
  • indi3rdParty.indi-webcam
  • indi3rdParty.indi-weewx-json
  • kstars
  • phd2

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Apr 24, 2024
@ofborg ofborg bot requested review from hjones2199 and sheepforce April 24, 2024 23:21
sheepforce
sheepforce approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@sheepforce sheepforce left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh wow, this looks so much better and cleaner! 😍 🐑 Thank you! Do you have any idea if r-ryantm updates work on them correctly now?

@returntoreality
Copy link
Contributor Author

Thanks! Regarding the r-ryantm bot, I don't think it fixes it. I think indilib needs a passthru.updateScript that also updates the indi-3rdparty repo. I wanted to look into this after these changes are merged. Since the 23.05 release is soon no breaking changes are supposed to be merged (I think this would probably count as one).

@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Apr 26, 2024
@returntoreality returntoreality force-pushed the indi-3rdparty-refactor branch 2 times, most recently from df3225f to 06334a1 Compare April 28, 2024 00:23
@ofborg ofborg bot requested a review from sheepforce April 28, 2024 00:51
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Apr 28, 2024
@Aleksanaa Aleksanaa changed the title Draft: indi-full: Indi 3rdparty refactor indi-full: Indi 3rdparty refactor Apr 29, 2024
@Aleksanaa Aleksanaa marked this pull request as draft April 29, 2024 13:48
@sheepforce sheepforce mentioned this pull request Jun 3, 2024
Merged
@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Jun 15, 2024
@returntoreality returntoreality marked this pull request as ready for review June 15, 2024 19:51
@returntoreality
Copy link
Contributor Author

I think it can be merged now, I rebased it to the current master.

@returntoreality returntoreality force-pushed the indi-3rdparty-refactor branch 2 times, most recently from 8fd25c0 to 997dd7c Compare June 15, 2024 20:26
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation 8.has: changelog labels Jun 15, 2024
sheepforce
sheepforce reviewed Jun 17, 2024
View reviewed changes
Copy link
Member

@sheepforce sheepforce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately there seems to be a build failure in ofBorg. Otherwise looks very nice!

JohnRTitor
JohnRTitor reviewed Jun 23, 2024
View reviewed changes
Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, could you rebase this? and this seems to fail on aarch64-linux?

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2024
@returntoreality returntoreality force-pushed the indi-3rdparty-refactor branch 2 times, most recently from f3bbf21 to 10e2a61 Compare July 29, 2024 21:13
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 29, 2024
@ofborg ofborg bot requested a review from sheepforce July 29, 2024 21:55
sheepforce
sheepforce suggested changes Aug 5, 2024
View reviewed changes
Copy link
Member

@sheepforce sheepforce left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a very good idea overall, thank you!

Two (hopefully) minor things left:

I think we should merge this, after these two are resolved :)

@returntoreality returntoreality force-pushed the indi-3rdparty-refactor branch 4 times, most recently from 59c4f2d to 74fdd39 Compare August 17, 2024 18:59
@ofborg ofborg bot requested a review from sheepforce August 17, 2024 19:06
@returntoreality
indi-full: refactor 3rdparty drivers
d2367c3 
This splits the 3rdparty drivers into seperate
packages as recommended by upstream. This also
allows to build a indi-full equivalent with only
the needed drivers. Also add indi-full-nonfree
with all the nonfree drivers. And remove them
from indi-full.
@returntoreality
Copy link
Contributor Author

Thanks for the reviews!

Two (hopefully) minor things left:
* There is a merge conflict to be resolved

This is solved now, moved the changelog entry to a random position so hopefully it won't have a conflict anytime soon.

* It fails on aarch64-linux as it tries to compile with Raspberry Pi GPIO support. Ofborg has the error log here https://logs.ofborg.org/?key=nixos/nixpkgs.306650&attempt_id=c546a1ea-0537-4f2e-b08c-f8d3dd622a59 and it looks like `pigpiod_if2.h` is missing. I'm not sure whether this is because the builder is not a RPi or if there is actually a dependency not declared?

I disabled the driver, this apparently relies on the pigpio library/service which is not avalable in nixpkgs currently.

I think we should merge this, after these two are resolved :)

That would be great!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4464

@JohnRTitor JohnRTitor merged commit 8da188f into NixOS:master Aug 25, 2024
31 checks passed
@returntoreality returntoreality deleted the indi-3rdparty-refactor branch August 26, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnRTitor JohnRTitor JohnRTitor left review comments

@hjones2199 hjones2199 Awaiting requested review from hjones2199

@sheepforce sheepforce Awaiting requested review from sheepforce

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@returntoreality @nixos-discourse @sheepforce @JohnRTitor @wegank